Skip to content

improve prompt caching by computing a deterministic hash (+ other bug fixes)#2509

Merged
sawka merged 5 commits intomainfrom
sawka/deterministic-hashing
Nov 3, 2025
Merged

improve prompt caching by computing a deterministic hash (+ other bug fixes)#2509
sawka merged 5 commits intomainfrom
sawka/deterministic-hashing

Conversation

@sawka
Copy link
Member

@sawka sawka commented Nov 3, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 3, 2025

Walkthrough

Adds deterministic 8-character suffix generation in Go by introducing generateDeterministicSuffix(inputs ...string) (SHA-256 + hex) and replaces prior random suffixes for AttachedTextFile and AttachedDirectoryListing with suffixes derived from content+filename or content+dirname. Adds imports crypto/sha256 and encoding/hex. Separately, in the Electron layer adds a synchronous IPC channel "get-home-dir" in the main process, exposes getHomeDir via the preload API as a synchronous IPC call, and updates the ElectronApi type to include getHomeDir(): string. In the frontend, frontend/app/view/codeeditor/schemaendpoints.ts introduces helpers (prependWildcard, convertToTildePath, makeConfigPathMatches) and replaces static per-endpoint fileMatch arrays with dynamically generated patterns (including wildcard and tilde variants). No exported Go function signatures were changed.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Inspect generateDeterministicSuffix for correct concatenation, encoding, and consistent 8-char selection.
  • Verify replacement sites for suffix usage in AttachedTextFile and AttachedDirectoryListing use the intended inputs and maintain previous uniqueness/collision expectations.
  • Check added Go imports for proper usage and gofmt/unused-import issues.
  • Review Electron main handler "get-home-dir" for sync IPC correctness and potential blocking implications.
  • Verify preload getHomeDir exposure matches IPC conventions and type expectations.
  • Ensure frontend/types/custom.d.ts addition aligns with the preload API and build/type checks.
  • Review frontend/app/view/codeeditor/schemaendpoints.ts changes thoroughly: correctness of makeConfigPathMatches, interactions with existing endpoint logic, edge cases for tilde expansion, and that generated fileMatch patterns match prior intended coverage.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided, making it impossible to assess whether it relates to the changeset. Add a pull request description explaining the purpose of the deterministic hashing changes and the nature of the other bug fixes included.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change (deterministic hash for prompt caching) and acknowledges additional fixes, which aligns with the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/deterministic-hashing

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d3549b and 5676f99.

📒 Files selected for processing (1)
  • frontend/app/view/codeeditor/schemaendpoints.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/view/codeeditor/schemaendpoints.ts (2)
frontend/app/store/global.ts (1)
  • getApi (826-826)
frontend/util/endpoints.ts (1)
  • getWebServerEndpoint (10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Build for TestDriver.ai
🔇 Additional comments (4)
frontend/app/view/codeeditor/schemaendpoints.ts (4)

13-15: LGTM - Wildcard prepending logic is correct.

The conditional logic correctly handles both absolute paths (starting with /) and relative/tilde paths by ensuring a slash is properly placed after the wildcard.


25-34: Previous review concern has been addressed.

The literal tildePath is now correctly included in the paths array (line 30) before its wildcarded variant (line 31), which resolves the issue flagged in the previous review where tilde-prefixed file opens would miss their schemas.


37-40: LGTM - Schema endpoint mappings are correctly configured.

The dynamic path generation via makeConfigPathMatches is applied consistently across all four schema endpoints with appropriate config path suffixes.


17-23: ****

The trailing slash edge case described in the review comment is not a real issue. The getHomeDir() implementation calls Electron's app.getPath("home"), which returns the home directory path without a trailing slash (e.g., /home/user, not /home/user/).

The convertToTildePath function therefore works correctly:

  • If absolutePath="/home/user/.waveterm" and homeDir="/home/user", then slice(homeDir.length) produces "/.waveterm", and the result is correctly "~/.waveterm".
  • If absolutePath equals homeDir exactly, slice() produces an empty string, and the result is correctly "~".

The proposed diff is unnecessary. The code is correct as-is.

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
pkg/aiusechat/openai/openai-convertmessage.go (2)

393-399: Consider extracting hash generation to a helper function.

The deterministic hashing logic works correctly for prompt caching. However, this same pattern is duplicated for directory listings (lines 422-428).

Consider extracting to a helper function:

// computeDeterministicSuffix generates an 8-character deterministic suffix
// from content and name using SHA-256 hashing for prompt caching
func computeDeterministicSuffix(content, name string) string {
	hasher := sha256.New()
	hasher.Write([]byte(content))
	hasher.Write([]byte(name))
	hash := hasher.Sum(nil)
	return hex.EncodeToString(hash)[:8]
}

Then use it as:

-		// Generate deterministic suffix from content hash for prompt caching
-		hasher := sha256.New()
-		hasher.Write([]byte(textContent))
-		hasher.Write([]byte(fileName))
-		hash := hasher.Sum(nil)
-		deterministicSuffix := hex.EncodeToString(hash)[:8]
+		// Generate deterministic suffix from content hash for prompt caching
+		deterministicSuffix := computeDeterministicSuffix(textContent, fileName)

Note on collision risk: The 8-character truncation (32 bits) provides ~4 billion unique values. This should be sufficient for typical conversation contexts, but consider increasing to 12-16 characters if you observe collisions in production.


422-428: Duplicate hashing logic - use the same helper function.

This is identical to the text file hashing logic above (lines 393-399). If you extract the helper function suggested in the previous comment, apply it here as well:

-		// Generate deterministic suffix from content hash for prompt caching
-		hasher := sha256.New()
-		hasher.Write([]byte(jsonContent))
-		hasher.Write([]byte(directoryName))
-		hash := hasher.Sum(nil)
-		deterministicSuffix := hex.EncodeToString(hash)[:8]
+		// Generate deterministic suffix from content hash for prompt caching
+		deterministicSuffix := computeDeterministicSuffix(jsonContent, directoryName)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8768269 and 20fdac1.

📒 Files selected for processing (1)
  • pkg/aiusechat/openai/openai-convertmessage.go (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Build for TestDriver.ai
🔇 Additional comments (1)
pkg/aiusechat/openai/openai-convertmessage.go (1)

9-9: LGTM!

The addition of crypto/sha256 and encoding/hex imports is appropriate for implementing deterministic hash-based suffixes.

Also applies to: 11-11

@sawka sawka changed the title improve prompt caching by computing a deterministic hash improve prompt caching by computing a deterministic hash (+ other bug fixes) Nov 3, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 43b6383 and 9d3549b.

📒 Files selected for processing (1)
  • frontend/app/view/codeeditor/schemaendpoints.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/view/codeeditor/schemaendpoints.ts (2)
frontend/app/store/global.ts (1)
  • getApi (826-826)
frontend/util/endpoints.ts (1)
  • getWebServerEndpoint (10-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Build for TestDriver.ai

@sawka sawka merged commit 956bf95 into main Nov 3, 2025
7 of 8 checks passed
@sawka sawka deleted the sawka/deterministic-hashing branch November 3, 2025 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant